-
Notifications
You must be signed in to change notification settings - Fork 6
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
UI series #606
UI series #606
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Make sure to pass build
job in CI before requesting review.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Apart from the comments in codes,
- Remove commented codes.
- Use
readonly
if possible. - Use
undefined
instead ofnull
if possible.
src/app/features/home/post-capture-tab/post-capture-tab.component.ts
Outdated
Show resolved
Hide resolved
src/app/features/home/post-capture-tab/post-capture-tab.component.scss
Outdated
Show resolved
Hide resolved
src/app/shared/services/dia-backend/series/dia-backend-series-repository.service.ts
Outdated
Show resolved
Hide resolved
src/app/shared/services/dia-backend/series/dia-backend-series-repository.service.ts
Outdated
Show resolved
Hide resolved
src/app/shared/services/dia-backend/series/dia-backend-series-repository.service.ts
Show resolved
Hide resolved
src/app/shared/services/dia-backend/series/dia-backend-series-repository.service.ts
Outdated
Show resolved
Hide resolved
src/app/shared/services/dia-backend/series/dia-backend-series-repository.service.ts
Outdated
Show resolved
Hide resolved
src/app/features/home/post-capture-tab/post-capture-tab.component.ts
Outdated
Show resolved
Hide resolved
id: string | null | undefined; | ||
cover: string | null | undefined; | ||
collectionGeneral = [ | ||
{ img: null }, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Replace null
with undefined
if possible.
@@ -0,0 +1,52 @@ | |||
// tslint:disable: no-magic-numbers |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why mark this "unresolved" converation resolved? Still, you did not remove the deprecated comments for TSLint.
src/app/shared/services/dia-backend/series/dia-backend-series-repository.service.ts
Outdated
Show resolved
Hide resolved
src/app/shared/services/dia-backend/series/dia-backend-series-repository.service.ts
Outdated
Show resolved
Hide resolved
@CS6 Remember to pass all checks before requesting for review. Thanks. |
@@ -0,0 +1,52 @@ | |||
// tslint:disable: no-magic-numbers |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why mark this "unresolved" converation resolved? Still, you did not remove the deprecated comments for TSLint.
關於頁面 3 的 C 部分 (按照 asset 是否擁有的狀況顯示 asset 或者底圖),這個部分等 QA 測完第一部分後,會請 @seanwu1105 接手
換句話說,QA 測是你的 release 會期待
* 可以從 photo view 切換到 series view 再切回來,都不會有問題,可以正確看到 買過的 series
* 點選 series 封面會進入 collection view
* 在 collection view 底下,再點選上方的 series view icon 會回到 series view
此部分之前已改為開新頁面顯示
* 在 collection view 底下,可以看到正確的 series 封面與留空的九宮格照片,但裡面目前並無法正確顯示照片
* 在 collection view 滑動的時候,封面照片會跟著滑動
New series 封面樣式